Reject NaN against a declared float bound - #280
Open
dylanpulver wants to merge 1 commit into
Open
Conversation
is_float compared with `value < min_val` / `value > max_val`. Every comparison against NaN is False, so a NaN satisfied neither test and was returned as valid: `float(0, 10)` accepts the string 'nan', while 'inf' and '-inf' are correctly rejected by those same bounds. docs/validate.rst describes these parameters as "any value from 3 to 9" (integer, and float "has the same parameters"), which NaN is not. Phrase the checks as "must satisfy the bound" instead, so NaN fails them. An unbounded `float` still accepts NaN, unchanged. is_integer is unaffected: it converts through int() first, which rejects NaN before any bound is compared.
Collaborator
|
Please keep PR descriptions brief and to the point. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
is_floatbounds-checks withvalue < min_val/value > max_val(
src/configobj/validate.py:843-846). Every comparison against NaN is False, soa NaN violates neither test and is returned as valid — while
infand-infare correctly rejected by those same bounds:
docs/validate.rstdescribes these parameters asinteger(3, 9) # any value from 3 to 9, with float having "the same parameters as the integer check".NaN is not a value from 3 to 9.
The fix phrases each check as must satisfy the bound rather than must not
violate it, so NaN fails. Scope is deliberately narrow:
floatstill accepts NaN — unchanged, and pinned by a testis_integeris unaffected: it converts throughint(), which rejects NaNbefore any bound is compared (verified)
Remedy choice worth a second opinion: with a bound declared, NaN now raises
VdtValueTooSmallError(orVdtValueTooBigErrorfor a max-only spec). That isthe natural consequence of the ordering rather than a considered claim that NaN
is "too small". If you'd rather it raised
VdtTypeError, say so and I'll changeit — I avoided that as the default because the obvious form of it (reject NaN
whenever it appears) also breaks unbounded
float, which nothing asked for.Test runs (same command and environment each time)
python -m pytest src/tests/ -q, based on5.0.x:5.0.x76 passed5.0.x+ the new tests2 failed, 78 passed80 passedI also built the blanket version (
if value != value: raise VdtTypeErrorregardless of bounds):
3 failed, 77 passed— it failstest_unbounded_float_is_unchanged, which is why that test is there.Based on
5.0.xrather than the defaultreleasebranch, since #275, #276 and#278 all merged there. Prior art: no open PR touches
is_float(checked thechanged files of all 14 open PRs; #267 and #127 touch
validate.py, for typehints and strict-spec validation).
AI disclosure: found and drafted with Claude Code (model Claude Opus 5,
claude-opus-5) — probing the numeric validators with non-comparable valuesafter noticing
infwas caught and NaN was not. Reviewed before submission.